Conversation
Register ThrottlerModule globally with configurable default limits (100 req/60s). Apply stricter per-endpoint limits on auth routes via @Throttle decorator: - POST /auth/login: 10 req/60s (AUTH_LOGIN_THROTTLE_LIMIT/TTL) - POST /auth/register: 5 req/60s (AUTH_REGISTER_THROTTLE_LIMIT/TTL) - POST /auth/forgot-password: 5 req/60s (AUTH_FORGOT_THROTTLE_LIMIT/TTL) All limits are env-configurable. ThrottlerGuard applied as global APP_GUARD. 429 responses include Retry-After header via ThrottlerGuard defaults. Closes #95
There was a problem hiding this comment.
Pull request overview
This PR adds application-level rate limiting using @nestjs/throttler, with a global default throttle and tighter limits on high-risk authentication endpoints to mitigate brute force and abuse.
Changes:
- Add
@nestjs/throttlerdependency. - Register
ThrottlerModuleglobally and applyThrottlerGuardas a globalAPP_GUARD. - Add per-route
@Throttle()overrides for/auth/login,/auth/register, and/auth/forgot-password, and document env vars in.env.example.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Locks @nestjs/throttler dependency resolution. |
| backend/package.json | Adds @nestjs/throttler dependency. |
| backend/src/app.module.ts | Configures global throttler defaults and installs ThrottlerGuard globally. |
| backend/src/modules/auth/auth.controller.ts | Adds stricter per-endpoint throttling for sensitive auth routes. |
| backend/.env.example | Documents new rate-limit env variables and defaults. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @Throttle({ | ||
| default: { | ||
| ttl: parseInt(process.env['AUTH_LOGIN_THROTTLE_TTL'] ?? '60000'), | ||
| limit: parseInt(process.env['AUTH_LOGIN_THROTTLE_LIMIT'] ?? '10'), | ||
| }, |
There was a problem hiding this comment.
The per-route throttling config reads directly from process.env and uses parseInt without validation. If an env var is set to a non-numeric value, parseInt can yield NaN or partially-parse (e.g. '10s' -> 10), which can cause unexpected throttling behavior at runtime. Consider parsing with Number(...) + Number.isFinite checks (or a small helper) and failing fast on invalid config, and prefer using ConfigService-derived constants for consistency with the global throttler config.
| @Throttle({ | ||
| default: { | ||
| ttl: parseInt(process.env['AUTH_REGISTER_THROTTLE_TTL'] ?? '60000'), | ||
| limit: parseInt(process.env['AUTH_REGISTER_THROTTLE_LIMIT'] ?? '5'), | ||
| }, |
There was a problem hiding this comment.
Same issue as the login throttle block: env parsing here can silently produce NaN/incorrect values (parseInt partial parsing, no radix/finite checks). Please centralize and validate these numeric configs (e.g., parse once at module load) so invalid env configuration fails fast and behavior is predictable.
| @Throttle({ | ||
| default: { | ||
| ttl: parseInt(process.env['AUTH_FORGOT_THROTTLE_TTL'] ?? '60000'), | ||
| limit: parseInt(process.env['AUTH_FORGOT_THROTTLE_LIMIT'] ?? '5'), | ||
| }, |
There was a problem hiding this comment.
Same env parsing/validation concern for forgot-password throttling: parseInt on env vars can yield NaN or partially-parsed numbers. Recommend validating these values and/or sourcing them via ConfigService-derived constants to avoid misconfiguration leading to unexpected throttle windows/limits.
| @Throttle({ | ||
| default: { | ||
| ttl: parseInt(process.env['AUTH_LOGIN_THROTTLE_TTL'] ?? '60000'), | ||
| limit: parseInt(process.env['AUTH_LOGIN_THROTTLE_LIMIT'] ?? '10'), | ||
| }, | ||
| }) | ||
| @UseGuards(LocalAuthGuard) | ||
| @Post('login') | ||
| async login(@Request() req: ExpressRequest) { |
There was a problem hiding this comment.
New rate-limiting behavior on auth endpoints isn’t covered by tests yet. Since there’s already extensive e2e coverage for /auth routes, it would be valuable to add an e2e case that exceeds the limit and asserts a 429 response (and presence/format of Retry-After if expected).
| useFactory: (configService: ConfigService) => [ | ||
| { | ||
| name: 'default', | ||
| ttl: configService.get<number>('THROTTLE_TTL', 60000), | ||
| limit: configService.get<number>('THROTTLE_LIMIT', 100), | ||
| }, | ||
| ], |
There was a problem hiding this comment.
ThrottlerModule is configured from ConfigService, but these values may be strings when sourced from environment variables. To prevent passing string/NaN values into the throttler config, parse/validate THROTTLE_TTL and THROTTLE_LIMIT explicitly (e.g., Number(...) + finite checks) before returning them from the factory.
| useFactory: (configService: ConfigService) => [ | |
| { | |
| name: 'default', | |
| ttl: configService.get<number>('THROTTLE_TTL', 60000), | |
| limit: configService.get<number>('THROTTLE_LIMIT', 100), | |
| }, | |
| ], | |
| useFactory: (configService: ConfigService) => { | |
| const throttleTtl = Number( | |
| configService.get<string | number>('THROTTLE_TTL', 60000), | |
| ); | |
| const throttleLimit = Number( | |
| configService.get<string | number>('THROTTLE_LIMIT', 100), | |
| ); | |
| return [ | |
| { | |
| name: 'default', | |
| ttl: Number.isFinite(throttleTtl) ? throttleTtl : 60000, | |
| limit: Number.isFinite(throttleLimit) ? throttleLimit : 100, | |
| }, | |
| ]; | |
| }, |
| providers: [ | ||
| AppService, | ||
| { | ||
| provide: APP_GUARD, | ||
| useClass: ThrottlerGuard, | ||
| }, |
There was a problem hiding this comment.
Using the default ThrottlerGuard without proxy/trust configuration means the rate-limit key is typically derived from req.ip. If this service is deployed behind a reverse proxy/load balancer, all clients may appear to come from the proxy IP unless trust proxy/X-Forwarded-For handling is configured. Consider using a custom ThrottlerGuard (override tracker) and/or ensuring the underlying HTTP adapter is configured to trust the proxy in production.
| # Rate Limiting (TTL in milliseconds) | ||
| THROTTLE_TTL=60000 | ||
| THROTTLE_LIMIT=100 | ||
| AUTH_LOGIN_THROTTLE_TTL=60000 | ||
| AUTH_LOGIN_THROTTLE_LIMIT=10 | ||
| AUTH_REGISTER_THROTTLE_TTL=60000 | ||
| AUTH_REGISTER_THROTTLE_LIMIT=5 | ||
| AUTH_FORGOT_THROTTLE_TTL=60000 | ||
| AUTH_FORGOT_THROTTLE_LIMIT=5 |
There was a problem hiding this comment.
The example config calls these values “TTL in milliseconds”. To avoid misconfiguration, consider making the unit expectation unambiguous (e.g., rename vars to *_TTL_MS or adjust the comment to match the throttler’s expected units) and ensure the values used in AppModule/AuthController are consistent with that unit.
Summary
ThrottlerModuleglobally inAppModulewith configurable default limits (100 req / 60s viaTHROTTLE_LIMIT/THROTTLE_TTLenv vars)ThrottlerGuardas a globalAPP_GUARDso all endpoints are protected by default@Throttlelimits on sensitive auth routes:POST /auth/login: 10 req / 60s (AUTH_LOGIN_THROTTLE_LIMIT/AUTH_LOGIN_THROTTLE_TTL)POST /auth/register: 5 req / 60s (AUTH_REGISTER_THROTTLE_LIMIT/AUTH_REGISTER_THROTTLE_TTL)POST /auth/forgot-password: 5 req / 60s (AUTH_FORGOT_THROTTLE_LIMIT/AUTH_FORGOT_THROTTLE_TTL)Retry-Afterheader via ThrottlerGuard defaults.env.examplewith all new rate-limiting variablesTest plan
pnpm dev:backendPOST /auth/login11 times within 60 seconds — expect 429 on the 11th request with aRetry-AfterheaderGET /) still accept traffic up to the 100-req default limitAUTH_LOGIN_THROTTLE_LIMIT=3in.envand verify 429 occurs on the 4th requestCloses #95